feat(http): make a retried write safe to send twice - #281
Merged
Conversation
A client that loses the response to a request cannot know whether the request landed, and on a mobile network that is routine. Retrying is the only thing it can do, and today that retry is a second post, a second comment, a second uploaded file. An Idempotency-Key claims a slot through CachePort.setIfAbsent - one SET NX EX, so the store decides the race rather than the API reading and then writing - and a retry is answered from the first attempt instead of running again. Opt-in on the seven writes where a duplicate is real damage; a like, a follow and a device registration are already repeatable and pay nothing. The record key carries the account, because a key is a value the client invents and two people can pick the same one. It also carries a fingerprint of the body, so a key reused with a different request is refused rather than answered with the earlier result. It fails open: an unreachable cache logs and lets the request through. This is a safety net over a write that already works, and a hard dependency would turn a cache blip into nobody being able to post.
|
🎉 This PR is included in version 1.27.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the
[deferred] Idempotency-Key on write endpointsitem on the mobile roadmap. It was scheduled to land before Play Billing, where a duplicate matters far more than a duplicate post.Summary
A client that loses the response to a request cannot know whether the request landed. On a mobile network that is routine, and retrying is the only thing it can do — so today that retry is a second post, a second comment, a second uploaded file.
Send
Idempotency-Keyand the retry is answered from the first attempt instead of running again, withIdempotent-Replay: trueon the reply. Records live 24 hours in Redis. A request with no key behaves exactly as it did before, which is what leaves the web client untouched.The claim is a single
SET NX EXthrough the newCachePort.setIfAbsent, so the store settles the race rather than the API reading and then writing — the same "the write is the claim" shapeDigestDeliveryuses.Opt-in per route (
config: { idempotency: true }), on the seven writes where a duplicate is real damage: posts, both comment endpoints, articles, messages, and the two upload endpoints — uploads because they cost storage and moderation. Everything else is already repeatable: likes, follows and bookmarks are idempotent by nature, a report has a unique constraint, a device registration is an upsert. Wrapping those would buy a round trip and nothing else.Notable decisions
idem:v1:<userId>:<method>:<route>:<key>). A key is a value the client invents; two people picking the same one is ordinary, and a shared bucket would hand one of them the other's response.docs/idempotency.mdsays plainly that an endpoint which moves money should revisit that trade rather than inherit it.One bug worth naming
The first cut fingerprinted with
JSON.stringify(body, Object.keys(body).sort()), intending "key order should not matter". That second argument is a filter, applied at every depth — so any nested key absent from the top-level list is dropped, and two bodies differing only somewhere nested fingerprint identically. That is worse than missing a duplicate: it replays the wrong response to a genuinely different request. Replaced with a small recursive stable serialiser, and the three cases now have tests.Tests
tests/e2e/idempotency/idempotency.test.ts: the same key creating one post and replaying the answer; the same key with a different body refused; two accounts using one key kept apart; no key still creating two posts; and a rejected request not spending its key.tsc -p tsconfig.build.json --noEmit,eslint,prettier --checkclean.Nothing to configure — no new environment variables, and the behaviour is inert until a client sends the header.
AI Asistan: Opus 5